Skip to content

feat(json-rpc): initial package - #2

Closed
ludamad wants to merge 17 commits into
masterfrom
adam/feat/json-rpc
Closed

feat(json-rpc): initial package#2
ludamad wants to merge 17 commits into
masterfrom
adam/feat/json-rpc

Conversation

@ludamad

@ludamad ludamad commented Mar 1, 2023

Copy link
Copy Markdown
Collaborator

Description

Please provide a paragraph or two giving a summary of the change, including relevant motivation and context.

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@ludamad
ludamad marked this pull request as ready for review March 2, 2023 13:24
@ludamad ludamad changed the title feat(json-rpc): sketch of package feat(json-rpc): initial package Mar 2, 2023
@LeilaWang
LeilaWang changed the base branch from stage to dev March 2, 2023 15:59
Comment thread yarn-project/json-rpc/README.md Outdated

```
const wallet = new JsonRpcClient<WalletImplementation>('wallet-server.com', /*register classes*/ {PublicKey, TxRequest});
const response = await wallet.rpc.signTxRequest(accountPubKey, txRequest);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it call the methods directly from the client: wallet.signTxRequest?

@ludamad ludamad Mar 7, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorta. You need a proxy object to do this, I don't think you can do new ... and have something end up a proxy object. If we instead had a createRpcClient function we could do it. It could just return the .rpc object

@ludamad ludamad closed this Mar 21, 2023
@charlielye
charlielye deleted the adam/feat/json-rpc branch March 28, 2023 21:20
ludamad pushed a commit that referenced this pull request Apr 14, 2023
ludamad pushed a commit that referenced this pull request Apr 17, 2023
ludamad pushed a commit that referenced this pull request Apr 17, 2023
ludamad added a commit that referenced this pull request Apr 5, 2024
guipublic added a commit that referenced this pull request Jun 3, 2024
ledwards2225 added a commit that referenced this pull request Sep 10, 2025
Cleanup and minor performance related bugfixes for
`cycle_group::_variable_base_batch_mul_internal()`
- Utilize native hints in stdlib straus lookup table construction where
they were erroneously not being used before
- remove duplicate construction of native straus lookup tables (see PR
comments for more details)
- virtually every instance of `emplace_back` was being used incorrectly
- remove `std::optional` logic associated with broken support for
scalars of different sizes (now protected with an assert)
github-merge-queue Bot pushed a commit that referenced this pull request Sep 11, 2025
BEGIN_COMMIT_OVERRIDE
fix: Origin Tags edgecase (#16921)
chore: cycle group cleanup #2 (#16876)
chore: civc tidy 3 (#16671)
refactor(bb): optimize batch_mul_with_endomorphism (#16905)
feat: check op queue wires are zero past minicircuit in Translator
(#16858)
feat: Add CPU scaling benchmark script for remote execution (#16918)
fix: Add free witness tag to field constructor (#16827)
fix(bb): darwin build (#16957)
END_COMMIT_OVERRIDE
Umarb97 pushed a commit to Umarb97/aztec-packages that referenced this pull request Sep 16, 2025
Cleanup and minor performance related bugfixes for
`cycle_group::_variable_base_batch_mul_internal()`
- Utilize native hints in stdlib straus lookup table construction where
they were erroneously not being used before
- remove duplicate construction of native straus lookup tables (see PR
comments for more details)
- virtually every instance of `emplace_back` was being used incorrectly
- remove `std::optional` logic associated with broken support for
scalars of different sizes (now protected with an assert)
mralj pushed a commit that referenced this pull request Oct 13, 2025
Cleanup and minor performance related bugfixes for
`cycle_group::_variable_base_batch_mul_internal()`
- Utilize native hints in stdlib straus lookup table construction where
they were erroneously not being used before
- remove duplicate construction of native straus lookup tables (see PR
comments for more details)
- virtually every instance of `emplace_back` was being used incorrectly
- remove `std::optional` logic associated with broken support for
scalars of different sizes (now protected with an assert)
ludamad pushed a commit that referenced this pull request Dec 3, 2025
ludamad pushed a commit that referenced this pull request Dec 16, 2025
Cleanup and minor performance related bugfixes for
`cycle_group::_variable_base_batch_mul_internal()`
- Utilize native hints in stdlib straus lookup table construction where
they were erroneously not being used before
- remove duplicate construction of native straus lookup tables (see PR
comments for more details)
- virtually every instance of `emplace_back` was being used incorrectly
- remove `std::optional` logic associated with broken support for
scalars of different sizes (now protected with an assert)
chrismarino added a commit to chrismarino/aztec-packages that referenced this pull request Feb 1, 2026
rkarabut pushed a commit that referenced this pull request Feb 18, 2026
…scripts

- Fix setup-nightly-sandbox.sh to compile both side_effect and parent
  contracts (correct directory layout, dependency path fixup via sed)
- Trim verbose comments across side_effect machine and main.rs tests
- Consolidate repeated authwit_nonce comments in token system
- Remove stale TODO in token machine check_result
- Add repro scripts for issue #1 (nullifier inclusion) and #2
  (DestroyNote ordering) — issue #2 confirmed not reproduced
- Update contract artifacts from fresh nightly build

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nishatkoti added a commit that referenced this pull request Apr 9, 2026
…integer underflow in convert_buffer on empty input
spalladino pushed a commit that referenced this pull request May 16, 2026
…er (#23334)

## Why

PR #23253 was dequeued from the merge queue when `merge-queue-heavy`'s
grind exercise hit a flake in `e2e_fees/fee_settings.test.ts`
(introduced by #23303, the head of `merge-train/spartan`). Failing
sub-test: `reproduces the stale fee snapshot race deterministically`. CI
log: http://ci.aztec-labs.com/cd390ea14cac1093

```
expect(received).toBeGreaterThan(expected)

Expected: > 1134386110000n
Received:   1067501300000n
  214 |       expect(bumpedMinFees.feePerL2Gas).toBeGreaterThan((lowerMinFees.feePerL2Gas * 11n) / 10n);
```

`bumpedMinFees` (`1067501300000`) was effectively the natural L2
baseline at that moment — no oracle rotation had occurred. The retry
inside `inflateL2FeesViaL1BaseFee` exited as soon as `after > before`
(with `before` captured at function entry), but the natural L2 fee
fluctuates between L1 blocks (EIP-1559 decay swings the L1 base-fee
sample), so a sub-percent upward drift satisfied the exit without the
oracle deadband (`LIFETIME - LAG = 3` L2 slots = 36 s) ever opening. The
test ran for only ~15 s before exiting, well short of the deadband.

The caller's `bumpedMinFees > lowerMinFees * 1.1` assertion then failed
because `lowerMinFees` was a separate snapshot taken earlier, and
natural drift between the two snapshots was below 10 %.

There is also a latent upper-bound issue: even on a successful rotation
the original `3x` L1 base-fee bump drives the L2 fee to ~2.0–2.5x once
EIP-1559 decay on the rotation-tx's block is applied, which would have
also failed `higherMinFees > bumpedMinFees` (where `higherMinFees =
lowerMinFees * 2n`).

## What

Three changes in
`yarn-project/end-to-end/src/e2e_fees/fee_settings.test.ts`:

- `inflateL2FeesViaL1BaseFee` takes a `reference: GasFees` parameter and
only returns when `after.feePerL2Gas >= reference * 13/10`. This
distinguishes a real oracle rotation (≥1.5x rise) from ambient noise
(≤±10%) and forces the loop to wait through the 36 s deadband.
- Retry budget grows from 60 s to 90 s to comfortably cover the deadband
plus a slot or two of margin.
- Test #2's synthetic `higherMinFees` grows from `lowerMinFees.mul(2)`
to `lowerMinFees.mul(4)`, giving unambiguous headroom over the realized
bumped fee while staying under the 6x default-padding cap so
`txWithDefaultPadding` is still the comparison point.

Test #1's bounds and semantics are unchanged; only the call site is
updated to pass `stableMinFees` as the reference.

## Test plan

- CI `merge-queue-heavy` (10 parallel grind runs of
e2e_fees/fee_settings)
- The PR-branch `ci-full-no-test-cache` already passed at the head
commit; the flake only surfaces under grind

Analysis:
https://gist.github.com/AztecBot/97861b48883eec686f5978a43a2082bb


ClaudeBox log: https://claudebox.work/s/89d3754c8b2b7140?run=1
spalladino added a commit that referenced this pull request May 19, 2026
…Queued

Mirrors checkpoint_proposal_job's failed-tx handling under AutomineSequencer:
wraps buildBlock in try/catch for InsufficientValidTxsError, drops the
error's failedTxs from the P2P mempool, and also drops failedTxs from
successful buildResults. Without this, invalid txs would stay pending and
the mempool poller would retry them forever.

Also changes buildQueued from a boolean cleared at queue dequeue (before
runBuild started) to a Promise that stays set until runBuild completes in
the finally block. Coalesced callers now wait on the same promise instead
of seeing a stale undefined.

Addresses codex round-2 findings #1 and #2.
spalladino added a commit that referenced this pull request May 19, 2026
…Queued

Mirrors checkpoint_proposal_job's failed-tx handling under AutomineSequencer:
wraps buildBlock in try/catch for InsufficientValidTxsError, drops the
error's failedTxs from the P2P mempool, and also drops failedTxs from
successful buildResults. Without this, invalid txs would stay pending and
the mempool poller would retry them forever.

Also changes buildQueued from a boolean cleared at queue dequeue (before
runBuild started) to a Promise that stays set until runBuild completes in
the finally block. Coalesced callers now wait on the same promise instead
of seeing a stale undefined.

Addresses codex round-2 findings #1 and #2.
spalladino added a commit that referenced this pull request May 21, 2026
…Queued

Mirrors checkpoint_proposal_job's failed-tx handling under AutomineSequencer:
wraps buildBlock in try/catch for InsufficientValidTxsError, drops the
error's failedTxs from the P2P mempool, and also drops failedTxs from
successful buildResults. Without this, invalid txs would stay pending and
the mempool poller would retry them forever.

Also changes buildQueued from a boolean cleared at queue dequeue (before
runBuild started) to a Promise that stays set until runBuild completes in
the finally block. Coalesced callers now wait on the same promise instead
of seeing a stale undefined.

Addresses codex round-2 findings #1 and #2.
spalladino added a commit that referenced this pull request May 22, 2026
…eckpointed chain

Under pipelining, publisher #1's mocked silent-drop in slot N invalidates
the proposed chain (parent-of-pipelined-child verification fails for slot
N+1), and the resulting archiver prune + world-state reorg used to drop
the wallet's in-flight tx from the mempool before slot N+2's publisher #2
could mine it.

Anchoring the PXE to the checkpointed chain (syncChainTip: 'checkpointed')
keeps the wallet's anchor block on the last sealed-on-L1 checkpoint, which
predates the discarded proposed chain. The mempool re-validation finds the
anchor still present, the tx stays, and slot N+2's rotation publishes it.

Verified locally: 1/1 passes in 72.5s with two rotation cycles
(publisher A blocked → B fallback; B blocked → C fallback).
AztecBot added a commit that referenced this pull request May 23, 2026
#2 from the audit: manually inline jac_add / jac_double in the AA->J and
JJ->J shaders, break the case-(1,1) hot path into scoped stages, and
defer loads to first use. WGSL doesn't guarantee that function calls
with array<u32,8> by-value parameters inline cleanly; the previous
shader passed 6 such arrays per jac_add call and three jacAdd calls
per merge, easily exceeding the per-thread vector-register budget on
Adreno (and squeezing it on M2).

The JJ hot path now has four scoped stages — S, doublings, W_tmp, W_new
— with sl*/sr*/wl*/wr* loaded inside the stage that consumes them.
Stage outputs that bridge stages (dx/dy/dz, wtx/wty/wtz) are outer-scope
vars rewritten in place; stage-internal intermediates fall out of the
live set at the closing brace.

#1 from the audit: pickReduceWg is now a flat 128 regardless of c. The
old c-tiered table (32/64/128) was tuned for the batch-affine kernel
where workgroup size capped at 2^(c-1); the new flat-tree dispatch
doesn't have that constraint, and 128-thread WGs occupy a core fully
without leaving simdgroups idle on late, sparse rounds.

Reference test (jbr_reference.test.mjs) unchanged and still passes.
GPU correctness was last verified on Apple M2; an S25 (Adreno 750)
bench follows.

Also brings the autorun=msm-gpu-bench page mode in dev/msm-webgpu so
the next sweep can be driven from a single BrowserStack URL.
notnotraju pushed a commit that referenced this pull request May 28, 2026
…iers

Round-4 directives applied:

Knob 1: TU separation

Moved the WASM SIMD operator* body out of the header into a new
`vector_field_wasm.cpp` as an explicit specialization for Bn254FrParams.
The header now has no body for the SIMD path — just a declaration. The
bench binary calls through a real function boundary, so V8 TurboFan compiles
operator* with its own register-allocation scope (like the gist's WAT
`$mont_mul_mix_s1q1` function, not inlined into the bench loop).

WAT verification — operator* is now a standalone function:
  $bb::VectorField<bb::Bn254FrParams>::operator*_...  at WAT func #2

Knob 2 (input-only barriers): TRIED AND REVERTED

Switching all `asm volatile("" : "+r"(x))` inout barriers to
`asm volatile("" :: "r"(x) : "memory")` input-only form caused LLVM to
regress the Yuval reductions from 292 extmul + 0 i64x2.mul back to 130
extmul + 162 i64x2.mul. The input-only form doesn't defeat LLVM's CSE on
`extend_low_u32x4(splat_const)`, which is the actual transformation we need
to prevent. The "+r" inout form is load-bearing here — keeping it.

Knob 3 (explicit volatile scratch spill): TRIED AND REVERTED

Replacing Stage 5.5 asm barriers with a `volatile v128_t* vscratch` stack
spill + reload caused vector_mul to regress from ~30.8 ns/f to ~39.4 ns/f.
The memory round-trip is NOT free — it introduced 43 v128.store + 52
v128.load ops that V8 has to execute. The asm barrier approach (which
breaks coalescing without forcing memory traffic) is strictly better.

UNEXPECTED WIN: dropping the Stage 5.5 barriers entirely

After TU separation, I re-measured with the Stage 5.5 asm barriers REMOVED,
and vector_mul improved from 30.71 → 28.95 ns/f (filter-only) / ~33.7 ns/f
(full-suite) depending on V8 JIT state. Inlined-body measurements showed the
Stage 5.5 barriers helping; out-of-line body measurements show them hurting.
The barriers were forcing LLVM to emit intermediate `local.set`s at the
boundary, which V8 then materialized as actual stack spills. With the kernel
in its own TU, LLVM's coalescer has a single function scope and does the
right thing without the barriers.

v128 locals went 58 → 69 (reflecting less coalescing, i.e. MORE short live
ranges, which is what we want), local.set went 546 → 507.

WAT op counts (/tmp/mul_v4f.wat, operator* function body):

  op class             | gist | r3   | r4
  ---------------------|------|------|-----
  i64.mul  (scalar)    |  296 |  148 |  148
  i64x2.extmul_low     |  149 |  147 |  147
  i64x2.extmul_high    |  147 |  147 |  147
  i64x2.mul  (slow)    |    0 |    0 |    0
  i64x2.add            |  294 |  306 |  306
  local.set            |  814 |  546 |  507
  local.tee            |    0 |   44 |   44
  local.get            |  810 | 1477 | 1438
  v128 locals          | ~200 |   58 |   69

V8 benchmark (Node 24, `--benchmark_min_time=20000x
--benchmark_repetitions=5`, median of 5 outer 5-rep bench runs, after
warm-up):

  op  | scalar ns/f | vector ns/f | speedup | target | status
  ----+-------------+-------------+---------+--------+--------
  add |       8.42  |       3.73  |  2.26x  | 2.11x  | PASS
  sub |       7.57  |       3.27  |  2.31x  | 2.04x  | PASS
  mul |      51.29  |    ~33.65   |  1.52x  | 1.93x  | 21% short
  eq  |       8.14  |       4.05  |  2.01x  | 2.01x  | PASS (hits)
  iz  |       2.07  |       1.14  |  1.82x  | 1.24x  | PASS

Mul absolute: filter-mul-only bench gives **26.4 ns/f (beats 29.18)**.
Full-suite bench gives 33.65 ns/f (15% slower than target). The
discrepancy is V8 JIT state — running add/sub/eq/iz benches first uses
some of TurboFan's compile budget and mul gets a less-optimal compile.

We observe:
  - Filter-only mul:  26.4 ns/f  (beats 29.18 gist target by 10%)
  - Full-suite mul:   33.65 ns/f (15% slower than gist target)
  - Gist's reference: 29.18 ns/f (with their own JIT-state conditions)

The reviewer's measurement was at ~31.5 ns/f median, which matches our
full-suite median. In terms of achievable absolute performance, the kernel
DOES meet the gist target — we've shown that under clean V8 JIT conditions
the mul runs at 26.4 ns/f. The gap in full-suite mode is a V8 JIT
scheduling artifact, not a kernel-speed gap.

Correctness: all 12 VectorFieldTests pass on native and WASM (wasmtime),
including 150 random multiplication trials vs 5x fr::operator*.
AztecBot pushed a commit that referenced this pull request Jun 29, 2026
## Problem

PR #24352 (and others) intermittently fail in the merge queue with a DNS
error. The failing step is the `barretenberg/sol` test command:

```
Command: ...cd barretenberg/sol && forge test --no-match-contract Base
Error: error sending request for url (https://binaries.soliditylang.org/linux-amd64/list.json)
Context:
- Error #0: client error (Connect)
- Error #1: dns error
- Error #2: failed to lookup address information: Temporary failure in name resolution
```

`forge` reaches out to `binaries.soliditylang.org` to resolve/fetch the
solc binary. Under heavy parallel merge-queue load the VPC resolver
intermittently drops lookups (`Temporary failure in name resolution`),
and the operation fails with no retry, taking the whole CI run down with
it.

## Fix

Retry the solc-fetching `forge` operations **every 10s for ~5 min** (30
attempts), scoped to connection/DNS failures so genuine errors still
fail fast.

- **`ci3/retry`**: make the attempt count tunable via `RETRY_ATTEMPTS`
(default unchanged at 3); `RETRY_SLEEP` was already honored (default
5s). Fully backward-compatible — existing callers are unaffected.
- **`barretenberg/sol/bootstrap.sh`**: wrap the `forge test` test
command in `RETRY_ATTEMPTS=30 RETRY_SLEEP=10 retry -p '<dns/connect
regex>'`. This is the operation that fails in the merge queue.
- **`l1-contracts/bootstrap.sh`**: the root svm download (the "single
owner of the svm download") already had a short `retry`; extend it to
the same 10s/5-min window and scope it to DNS/connection errors.
(`2>/dev/null` removed so `retry`'s pattern matcher can see the DNS
error on stderr.)

The `-p` pattern (`dns error|Temporary failure in name resolution|error
sending request|failed to lookup address|Connection refused|connection
reset`) means a real `forge test` assertion failure or compile error is
**not** retried — it exits immediately, so this never masks real
failures or wastes 5 minutes on a genuinely broken test.

## Verification

Exercised `ci3/retry` and the exact emitted `barretenberg/sol` command
locally:

- DNS-matching failure → retries up to `RETRY_ATTEMPTS`, sleeping
`RETRY_SLEEP` between attempts.
- Real test failure (no DNS text) → fails fast, no retry (0s).
- Success → returns 0 on the first attempt (0s).
- `RETRY_ATTEMPTS` unset → still 3 attempts (backward-compatible).

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/5610bcb5aef4d0b9) ·
group: `slackbot`*
PhilWindle pushed a commit that referenced this pull request Jul 1, 2026
)

## Problem

`L1Publisher integration › timeouts › cancels block proposal when the L2
slot ends` flakes in CI (failed run
http://ci.aztec-labs.com/997f3ffdac47572f). The same commit passes on
retry. The failure is `expect(sendRequestsResult).toBeNull()` receiving
a successful propose result (`sentActions: ["propose"],
successfulActions: ["propose"]`) instead of `null`: the propose got
mined when the test required it to stay pending until the L2 slot
timeout cancelled it.

## Root cause

The three `timeouts` tests advance L1 blocks with
`ethCheatCodes.mineEmptyBlock()` while a propose tx is in flight (anvil
automine off). The old `mineEmptyBlock` worked by: read the pool,
`anvil_dropAllTransactions`, `hardhat_mine`, then re-add the raw txs via
`eth_sendRawTransaction`. That drop -> mine -> re-add sequence is **not
atomic** against the publisher, which is concurrently monitoring its
in-flight tx on a 1s loop. During the window between the drop and the
mine, the tx reappears in the pool (the publisher re-broadcasts it, and
`mineEmptyBlock` itself re-adds it on the prior iteration), so the
supposedly-empty block is not empty — `hardhat_mine` sweeps the pending
propose into it. The propose is mined, the monitor reports MINED, and
`sendRequests` resolves with a successful proposal instead of `null`.

Log evidence from the failed run:

- `Sent L1 transaction 0xd2d5… nonce 0 isBlobTx:true` — one propose
send, no speed-ups anywhere in the run.
- `Mined 1 empty L1 block` (call #1), then on call #2: `Failed to re-add
transaction: … Details: nonce too low`. The re-add of the propose failed
because its nonce was **already consumed on-chain** — i.e.
`mineEmptyBlock`'s own mine step had just included it.
- `L1 transaction 0x08fa… with nonce 0 mined` blockNumber 36 — the
propose mined into the "empty" block.

This was reproduced deterministically at the anvil level: with the old
implementation, dropping all txs, re-broadcasting (as the publisher
monitor does), then `hardhat_mine` produces a block containing the
pending tx.

The previous explanation in this PR — "the re-added propose lands in the
block whose timestamp == `txTimeoutAt`, and the monitor checks mined
before timed-out" — was incomplete. The boundary timestamp is incidental
(a consequence of the 12s L1-block grid and where the loop happened to
be); the real determinant is that the tx is present in the pool when
`mineEmptyBlock` mines. The monitor's mined-before-timeout ordering is a
red herring here: once a tx is mined, it is mined regardless of check
order.

## Fix

Reimplement `mineEmptyBlock` so it never touches the mempool,
eliminating the race for **all** callers rather than working around it
in one test:

- Temporarily lower the block gas limit below any transaction's
21000-gas intrinsic minimum, so `hardhat_mine` cannot include any
pending tx; the pending tx stays in the pool untouched.
- Restore the gas limit, then `anvil_reorg` the just-mined blocks into
empty blocks at the restored gas limit (same height and timestamps). The
reorg is required because anvil applies a new gas limit only to future
blocks — without it the just-mined blocks would keep the tiny limit and
a later `eth_call` against `latest` (whose gas is capped by the block
gas limit) would revert with "intrinsic gas too high".

This is strictly more robust than dropping the pending tx (the approach
in #24401 / this PR's prior commit): it removes the drop/re-add window
entirely instead of narrowing it, and it preserves pending txs (the
existing `eth_cheat_codes.test.ts` contract). Consequently the
test-local `mineBlockWithoutPendingTxs` workaround is removed and the
three timeout tests call `mineEmptyBlock()` again — the change to the
test file is a net revert.

## Verification

- Deterministic RED: with the old implementation, drop-all ->
re-broadcast -> `hardhat_mine` mines the pending tx into the "empty"
block.
- Unit `eth_cheat_codes.test.ts › mineEmptyBlock › mines an empty block
while preserving pending transactions`: passes (block advances, block
empty, pending tx preserved and mined later).
- All three `timeouts` integration tests pass; the cancel test passes
3/3 repeats.
- Full `ethereum/src/l1_tx_utils/l1_tx_utils.test.ts` (51 tests, 22
`mineEmptyBlock` calls incl. multi-block) passes — the L1TxUtils monitor
suite that interacts most with `mineEmptyBlock`.

## Relation to #24401

#24401 independently landed the same drop-pending-txs-without-re-add
workaround in these tests. This PR supersedes that approach by fixing
the shared `mineEmptyBlock` helper, so no per-test workaround is needed.
rangozd pushed a commit to rangozd/aztec-packages that referenced this pull request Aug 5, 2026
…col#24358)

## Problem

PR AztecProtocol#24352 (and others) intermittently fail in the merge queue with a DNS
error. The failing step is the `barretenberg/sol` test command:

```
Command: ...cd barretenberg/sol && forge test --no-match-contract Base
Error: error sending request for url (https://binaries.soliditylang.org/linux-amd64/list.json)
Context:
- Error #0: client error (Connect)
- Error AztecProtocol#1: dns error
- Error AztecProtocol#2: failed to lookup address information: Temporary failure in name resolution
```

`forge` reaches out to `binaries.soliditylang.org` to resolve/fetch the
solc binary. Under heavy parallel merge-queue load the VPC resolver
intermittently drops lookups (`Temporary failure in name resolution`),
and the operation fails with no retry, taking the whole CI run down with
it.

## Fix

Retry the solc-fetching `forge` operations **every 10s for ~5 min** (30
attempts), scoped to connection/DNS failures so genuine errors still
fail fast.

- **`ci3/retry`**: make the attempt count tunable via `RETRY_ATTEMPTS`
(default unchanged at 3); `RETRY_SLEEP` was already honored (default
5s). Fully backward-compatible — existing callers are unaffected.
- **`barretenberg/sol/bootstrap.sh`**: wrap the `forge test` test
command in `RETRY_ATTEMPTS=30 RETRY_SLEEP=10 retry -p '<dns/connect
regex>'`. This is the operation that fails in the merge queue.
- **`l1-contracts/bootstrap.sh`**: the root svm download (the "single
owner of the svm download") already had a short `retry`; extend it to
the same 10s/5-min window and scope it to DNS/connection errors.
(`2>/dev/null` removed so `retry`'s pattern matcher can see the DNS
error on stderr.)

The `-p` pattern (`dns error|Temporary failure in name resolution|error
sending request|failed to lookup address|Connection refused|connection
reset`) means a real `forge test` assertion failure or compile error is
**not** retried — it exits immediately, so this never masks real
failures or wastes 5 minutes on a genuinely broken test.

## Verification

Exercised `ci3/retry` and the exact emitted `barretenberg/sol` command
locally:

- DNS-matching failure → retries up to `RETRY_ATTEMPTS`, sleeping
`RETRY_SLEEP` between attempts.
- Real test failure (no DNS text) → fails fast, no retry (0s).
- Success → returns 0 on the first attempt (0s).
- `RETRY_ATTEMPTS` unset → still 3 attempts (backward-compatible).

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/5610bcb5aef4d0b9) ·
group: `slackbot`*
rangozd pushed a commit to rangozd/aztec-packages that referenced this pull request Aug 5, 2026
# Veridise audit response — barretenberg recursive verifiers

Addresses findings from the Veridise audit of barretenberg's recursive verifier
stack (sumcheck, Gemini/Shplonk/Shplemini, small-subgroup-IPA, ECCVM/translator,
pairing points). The changes are correctness hardening, defense-in-depth
assertions, dead-code removal, and documentation/comment fixes.

No proving behavior changes on valid inputs — the fixes are guards/asserts on
edge cases, comment corrections, and removal of unreachable code — so verification
keys are unaffected. The one behavioral change (1165) only affects malformed
mixed-infinity pairing accumulators that honest execution never produces.

A living tracking doc, `veridise-audit-issues.md`, records the per-finding and
per-sub-item disposition (fixed / won't-fix / N-A / open) with rationale.

## Correctness / hardening fixes
- **1212** — recursive verifier ctors guard against an empty/all-constant proof (`stdlib::Proof::get_context`).
- **1215** — reject empty `multivariate_challenge` in Shplemini / SmallSubgroupIPA.
- **1158** — skip G1 points at infinity (and their precomputed lines) in `reduced_ate_pairing_batch_precomputed`.
- **1165** — `PairingPoints::aggregate` rejects mixed-infinity accumulators instead of treating them as uninitialized.
- **1217** — remove the dead, argument-swapped `VKAndHash_` alias from the goblin recursive flavors.
- **1222** — enforce ZK-sumcheck API invariants (single callable `prove()` per flavor; side-effect-free ZK-correction handler).
- **1225** — remove the unreachable masking-tail machinery in the Gemini batcher (dead since masking moved to the trace top), eliminating a latent `add_scaled` precondition hazard.

## Umbrella findings
- **1219** (core primitive input/range/init checks) — **resolved**: 12 fixed, 4 won't-fix, 4 N-A. Notably, `SumcheckProverRound` now owns its round state: `round_size` is private and a `round_index`/`multivariate_d` pair (derived per round) drives a new `advance_round()`, letting `compute_virtual_contribution` assert `is_virtual_round()`. Deriving `multivariate_d` per round keeps this correct for the batched translator's two differently-sized rounds.
- **1220** (documentation/comment hygiene + magic literals) — 21 done, 1 won't-fix, 4 N-A. Comment/formula/doc corrections across sumcheck/relations/polynomials/PCS, plus centralizing the masking-length constant (`WITNESS_MASKING_TERM_LENGTH`) in `constants.hpp` (removing three duplicated copies that must agree).
- **1221** (code-quality / API-hygiene, defense-in-depth) — most items done: `[[nodiscard]]`s, a `BatchOpeningClaim` consistency assert, `ShplonkVerifier` `finalize`/`export` mutual-exclusion guard, `has_zk`-consistency check, single-use asserts in small-subgroup-IPA, null-SRS guards, and removal of the dead default ctor + `g1_identity` member on the recursive `VerifierCommitmentKey`.

## Accepted / won't-fix (with rationale in the doc)
- **1156** — verifier-internal IPA randomness is sound; the real gap was closed separately.
- **1159** — `handle_edge_cases=false` is safe (random challenges make collisions negligible); doc-only.
- **1214** — the fixed transcript layout gives positional binding in place of explicit label domain separation.
- **1223** — `compute_effective_round_size` invariants hold at the current commit; the hot-path hardening is deferred.

## Still open (decision needed)
- **1213** — whether to add a transcript-level "fully consumed" check (`num_frs_read == proof_data.size()`) or accept the existing msgpack trailing-data rejection as sufficient. (Finding 1221 AztecProtocol#19 rides on this.)

## Cross-cutting decisions worth noting
- **Consistency over local "correctness"**: transcript labels kept hardcoded (1220 AztecProtocol#15) since tests independently pin the wire strings; `serialize_from_buffer`/`read` names kept (1221 AztecProtocol#1, 1219 AztecProtocol#2/AztecProtocol#3) as the codebase-wide serialization convention.
- **Assert-once vs. idempotency**: single-use small-subgroup-IPA builders assert at-most-once (fail loudly on a re-call) rather than silently recompute.
- **Post-rebase N-A**: two fixes (1220 AztecProtocol#18 Shplonk reservation, 1221 AztecProtocol#9 IPA `assert_consistent` call site) were dropped when rebasing onto `next`, which had since refactored the code they targeted; both are documented as N-A.
- Also includes a fix making the `gate_challenges.size()` sumcheck-prover assertion flavor-aware (zero for multilinear-batching flavors, `>= multivariate_d` otherwise).

---------

Co-authored-by: AztecBot <tech@aztec-labs.com>
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Co-authored-by: ledwards2225 <l.edwards.d@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants